Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Predict2.0 #153

Closed
wants to merge 33 commits into from
Closed

Predict2.0 #153

wants to merge 33 commits into from

Conversation

Farnazmdi
Copy link
Collaborator

No description provided.

src/ODEmodel.jl Outdated Show resolved Hide resolved
@Farnazmdi
Copy link
Collaborator Author

#152

@aarmey
Copy link
Member

aarmey commented Feb 5, 2020

Ok, I'll take this from here.

@aarmey
Copy link
Member

aarmey commented Feb 6, 2020

Ok, made the changes here. I didn't do the notebooks, so please fix those. You just pass the time now, not a vector with the time in the second element.

@Farnazmdi
Copy link
Collaborator Author

Are we using any preallocation in ODEjac?
There is this error about that ODEjac cannot take Dual numbers:
image

I read that AD doesn't work with preallocated functions

@aarmey
Copy link
Member

aarmey commented Feb 6, 2020

Oops. Think I just fixed that.

@Farnazmdi
Copy link
Collaborator Author

diffCell still doesn't work
image

I found this:
link
It looks like there is nothing to work around it

@Farnazmdi
Copy link
Collaborator Author

Is this way more accurate than calculating all the points?
The behavior is less monotonic.

@aarmey
Copy link
Member

aarmey commented Feb 6, 2020

Post plot?

@Farnazmdi
Copy link
Collaborator Author

dox:
image

gemcitabine:
image

lapatinib:
image

paclitaxel:
image

@aarmey
Copy link
Member

aarmey commented Feb 6, 2020

Hmm... the output should be the same. I did change the time since you were indicating the 100th timepoint before, and now it's 100.0 hrs. Maybe you could figure out what time the 100th point is, and then set it to that?

@aarmey
Copy link
Member

aarmey commented Feb 6, 2020

Also adding more concentrations may help find the issue. Because there's oscillatory behavior the result doesn't have to be monotonic, but it should be locally smooth.

@Farnazmdi
Copy link
Collaborator Author

Of course. Thank you.

@Farnazmdi
Copy link
Collaborator Author

tmp
The animation looks cool! This is the plot for almost all times.

@Farnazmdi
Copy link
Collaborator Author

Could I keep it in the notebooks?

@Farnazmdi
Copy link
Collaborator Author

image
@aarmey I couldn't figure out what the problem is. Could you please let me know what I did wrong or what I should do to correct this?

@aarmey
Copy link
Member

aarmey commented Feb 19, 2020

This is related to my post on #general. Should be fixed now, but juptyerlab won't be available until I hear back from seasnet.

@Farnazmdi
Copy link
Collaborator Author

Thanks.

@Farnazmdi
Copy link
Collaborator Author

Sometimes the cell number becomes negative. I put an assertion for that, I will try to see if this used to happen before the change to the jacobian.

@Farnazmdi
Copy link
Collaborator Author

I guess there is something wrong with the Calculus.finite_difference() because as it is clear here, with exactly the same set of parameters, the number of cells is decreasing.
image

The y-axis is the number of cells, and the x-axis is a range of gamma_1.
image

@aarmey do you see any other point that might be going wrong here?

@Farnazmdi
Copy link
Collaborator Author

Something that looks weird is that with a higher concentration (this is in gemcitabine) the G2 rate progression -- which also happens to be division rate -- increases! I searched a little to see if I could find some evidence of this, but couldn't.

@Farnazmdi
Copy link
Collaborator Author

Actually, there is something wrong with the predict2(), it sometimes returns negative cell numbers.

@Farnazmdi
Copy link
Collaborator Author

Since the solution in predict() and predict2() is not the same, and the second one sometimes returns negative cell numbers, if you let me I will use the predict() and just take 1 time point. Is that alright?

@aarmey
Copy link
Member

aarmey commented Feb 21, 2020

Yep. In that case get rid of predict2

@Farnazmdi
Copy link
Collaborator Author

Some updates:

  1. paclitaxel and doxorubicin seem to work alright in terms of their gradient. Gemcitabine goes positive for two last concentrations. There should be something related to the fact that beta increases significantly over concentrations.

There are two errors now.

  1. For all of the sensitivity analyses, the only first cell # is some negative small number (like -0.002). I changed the range we have for the parameters in sensitivity analysis, but it just happens with any range.
  2. In lapatinib, the number of death phase steps in G1 (nD1) is estimated to be zero. This messes with something that it errors for invalid array dimensions. It occurs when it reaches the line here:

image

@Farnazmdi
Copy link
Collaborator Author

This is the error regarding invalid array dimensions.
image

@Farnazmdi
Copy link
Collaborator Author

@aarmey I can't figure out what array dimension is wrong. Could you pleeease take a look at this error? When nD1 is estimated to be 0 for lapatinib, this error happens. I mean when the matrix dimension for death in G1 becomes zero. The fitie_difference breaks in this. I tried a few tricks to just remove that parameter, but it breaks other things. I would really appreciate if you could take a look at this. Any recommendations helps.
image

for ii = 1:length(G1)
G1[ii] = sum(v[1:nG1]) + sum(v[(nG1 + nG2 + 1):(nG1 + nG2 + nD1)])
G2[ii] = sum(v[(nG1 + 1):(nG1 + nG2)]) + sum(v[(nG1 + nG2 + nD1 + 1):(nG1 + nG2 + nD1 + nD2)])
if nD1 == 0 & nD2 != 0
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need an if statement here? If nD1 is equal to 0 doesn't it just end up having no effect on addition?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In either of G1 or G2, the second term sum(...) would be meaningless, for example in G1, since it is summing the index nG1+nG2+1 : nG1+nG2 if nD1= 0.

@aarmey
Copy link
Member

aarmey commented Feb 23, 2020

If we're not ultimately using predict2, then what do we need to keep from this branch?

@Farnazmdi
Copy link
Collaborator Author

I was trying to address the issues in the gradient here, but you are right, I could do it in another branch.

@Farnazmdi
Copy link
Collaborator Author

This error I mentioned still exists even with what it is in the branch master. It is for the control trial in lapatinib.

@aarmey
Copy link
Member

aarmey commented Feb 23, 2020

Yes, if there's nothing you need here, it'd be helpful for debugging to make a failing case off of master.

@Farnazmdi Farnazmdi closed this Feb 23, 2020
@aarmey aarmey deleted the predict2.0 branch February 23, 2020 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants